host-gcc: exclude -lgcc to fix -mx32 [qemu_]x86_64 regression #12805
Conversation
Codecov Report
@@ Coverage Diff @@
## master #12805 +/- ##
=======================================
Coverage 53.51% 53.51%
=======================================
Files 239 239
Lines 26989 26989
Branches 6521 6521
=======================================
Hits 14442 14442
Misses 9881 9881
Partials 2666 2666Continue to review full report at Codecov.
|
I'm quite confused about which toolchain configuration(s) x86_64 was originally tested with. This is why I didn't test x86_64 with ZEPHYR_TOOLCHAIN_VARIANT=host until after a chat with @andyross and let this regression slip. More details just added to PR #9522 @nashif would it be a lot of CI work to add a minimal "make run" / "ninja run" for all BOARD=qemu_* and how could I/we help? |
| # -march={pentium,lakemont,...} do not automagically imply -m32, so | ||
| # adding it here. | ||
|
|
||
| # There's only one 64bits ARCH (actually: -mx32). Let's exclude it to |
There was a problem hiding this comment.
nitpick: Until arm aarch64 is added :)
There was a problem hiding this comment.
... but not MIPS64? :-)
This is actually why I'm suggesting in the next paragraph to move this -m32 flag entirely out of this file in the longer term and spread them instead next to the corresponding -march flags. @SebastianBoe ?
This would be a significant refactoring though, way beyond the scope of this small regression fix.
| # -march={pentium,lakemont,...} do not automagically imply -m32, so | ||
| # adding it here. | ||
|
|
||
| # There's only one 64bits ARCH (actually: -mx32). Let's exclude it to |
There was a problem hiding this comment.
By x32 you mean the x32 ABI which has more registers than i386, right?
https://en.wikipedia.org/wiki/X32_ABI
But native_posix requires 32-bit always (so should be i386 compatible (and x32 I suppose)) and then you want to not set -m32 if we are on x86-64? What am I missing?
There was a problem hiding this comment.
Yes native_posix is neither 64 bits nor x32, which is why it's unrelated to this fix.
then you want to not set -m32 if we are on x86-64?
When compiling x86_64 with ZEPHYR_TOOLCHAIN_VARIANT=host right now, -m32 is (silently) overridden by -mx32 because -mx32 (luckily?) comes last. That doesn't seem right. This first part of the patch doesn't fix anything but it removes this element of luck.
x32 = qemu_x86_64 is what this fixes...
Well this is not Linux, is it? :-)
Thanks! I was actually wondering about something like that. Are you adding it to the Zephyr CI? :-) |
|
FWIW: -mx32 is an interrim solution for x86_64 that absolved me of the need to find and fix word size bugs in all of Zephyr. We don't care about anything but toolchain support for it, and gcc/binutils will continue to support it. The core ("xuk") of the x86_64 layer already works with native 64 bit code, so it's not like there are any huge roadblocks. Just need to turn it on and start fixing bugs. Maybe for 1.15. |
There was a problem hiding this comment.
This all looks correct to me modulo my limited abilities with cmake.
And I agree that we probably want some level of validation somewhere of ZEPHYR_TOOLCHAIN_VARIANT=host. It's the kind of thing that can bitrot quickly. Maybe throw one board into the slow CI runs that uses this and define our supported "host" as Ubuntu Xenial or whatever.
PR zephyrproject-rtos#9522 series ending with commit c2c9265 ("tests: cmsis: Disable two cmsis portability tests on x86_64") added -mx32 support for the x86_64 ARCH and qemu_x86_64. While this was implemented in "compiler/gcc/target.cmake" as fall back from cross-compilation to the host compiler, it worked with a direct ZEPHYR_TOOLCHAIN_VARIANT=host too. Later, -lgcc was added to "compiler/host-gcc/target.cmake" by PR zephyrproject-rtos#12674 to fix the -m32 x86 build. This broke the x86_64 build when using ZEPHYR_TOOLCHAIN_VARIANT=host because even "multilib" packages usually don't feature the -mx32 version of libgcc. Fix this by excluding -lgcc in compiler/host-gcc/target.cmake just like compiler/gcc/target.cmake always did for x86_64. Signed-off-by: Marc Herbert <[email protected]>
|
Pure rebase noise, zero code change. |
|
@marc-hb for the CI enhancements, open an issue/enhancement so we can track it |
|
btw, the way I test this: |
PR #9522 series ending with commit c2c9265 ("tests: cmsis: Disable
two cmsis portability tests on x86_64") added -mx32 support for the
x86_64 ARCH and qemu_x86_64. While this was implemented in
"compiler/gcc/target.cmake" as fall back from cross-compilation to the
host compiler, it worked with a direct ZEPHYR_TOOLCHAIN_VARIANT=host
too.
Later, -lgcc was added to "compiler/host-gcc/target.cmake" by PR #12674
to fix the -m32 x86 build. This broke the x86_64 build when using
ZEPHYR_TOOLCHAIN_VARIANT=host because even "multilib" packages usually
don't feature the -mx32 version of libgcc.
Fix this by excluding -lgcc in compiler/host-gcc/target.cmake just like
compiler/gcc/target.cmake always did for x86_64.
Signed-off-by: Marc Herbert [email protected]